Conversation
# Conflicts: # src/wp-admin/options-reading.php
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @pbearne@git.wordpress.org. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
…evelop into ticket/19958
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
| <?php _e( 'Your latest posts' ); ?> | ||
| <?php | ||
| $allowed_archives_on_home_page = array( 'posts') + $post_types_allowed_on_home_page; | ||
| unset( $allowed_archives_on_home_page['page'] ); |
There was a problem hiding this comment.
why are pages specifically excluded?
There was a problem hiding this comment.
This is a list of CPT that can be used as a home archive. This can not be pages
Pages shouldn't be set, but I'm protecting the settings here
There was a problem hiding this comment.
oh, got it. maybe add an inline comment to clarify. confused me when reviewing the code.
| $output = "<select name='show_on_front' id='homepage_types'>\n"; | ||
| $output .= sprintf( '<option value="page" %s>A static page (select below)</option>', selected( -1, strtolower( get_option( 'show_on_front' ) ), false ) ) ; | ||
| foreach ( $allowed_archives_on_home_page as $post_type ){ | ||
| $post_type = ( 'posts' === $post_type ) ? 'post' : $post_type; |
There was a problem hiding this comment.
This code came from the old code, so I kept it
There was a problem hiding this comment.
sure, its just ugly hence "oof". fine to keep.
| $output .= sprintf( '<option value="%s" %s>%s</option>', | ||
| $post_type, | ||
| selected( $post_type, strtolower( get_option( 'show_on_front' ) ),false ), | ||
| sprintf( __( 'Your latest %s' ), get_post_type_object( $post_type )->labels->name ) |
There was a problem hiding this comment.
Can we always assume get_post_type_object( $post_type )->labels->name exists? if not, maybe add some defensive checking before using.
There was a problem hiding this comment.
Looking at the code, it looks like it is always set.
src/wp-includes/class-wp-post-type.php:567
| printf( | ||
| /* translators: %s: Select field to choose the front page. */ | ||
| __( 'Homepage: %s' ), | ||
| wp_dropdown_pages( |
There was a problem hiding this comment.
do we use wp_dropdown_pages elsewhere? wondering out loud if we should update the function directly rather than replacing it here inline.
| * | ||
| * @see get_post_type_labels() for label keys. | ||
| */ | ||
| function wpdocs_codex_book_init() { |
There was a problem hiding this comment.
was this section for testing?
| add_action( 'init', 'twentytwentyfive_register_block_bindings' ); | ||
|
|
||
| /** | ||
| * Registers the 'books' custom post type. |
There was a problem hiding this comment.
looks lioke testing code that can be removed
| } | ||
|
|
||
| if ( $this->is_home && 'page' !== get_option( 'show_on_front' ) && empty( $query_vars['post_type'] ) ) { | ||
| $post_types_allowed_on_home_page = apply_filters( 'post_types_allowed_on_home_page', array_keys( get_post_types( array( 'show_in_home_page_list' => true ) ) ) ); |
There was a problem hiding this comment.
filters get documented once, other instances include a reference ( // This filter is documented in ...)
| $this->is_home = true; | ||
| } | ||
|
|
||
| if ( $this->is_home && 'page' !== get_option( 'show_on_front' ) && empty( $query_vars['post_type'] ) ) { |
There was a problem hiding this comment.
An inline comment explaining why this is being added here would be helpful (not obvious to me)
src/wp-includes/post.php
Outdated
| 'show_in_rest' => true, | ||
| 'rest_base' => 'posts', | ||
| 'rest_controller_class' => 'WP_REST_Posts_Controller', | ||
| 'show_in_home_page_list' => null, |
There was a problem hiding this comment.
Q: does this match the existing behavior (posts and pages are included)? I think it does, just checking.
adamsilverstein
left a comment
adamsilverstein
left a comment
There was a problem hiding this comment.
Overall looks good, I left some small feedback and there are also linter errors to fix. Can add tests to cover especially the wp-query change and the filter functionality? Also, testing instructions on the PR are always helpful (copy over from trac if already there). I think I know how to test this, but might miss some scenarios.
Co-authored-by: Adam Silverstein <adam@earthbound.com>
Co-authored-by: Adam Silverstein <adam@earthbound.com>
Co-authored-by: Adam Silverstein <adam@earthbound.com>
…update related logic.
|
Add a custom CPT (books) with the new arg 'show_in_home_page_list' => true, Change settings and check the home page changes as needed 
|
2 participants
Trac ticket: https://core.trac.wordpress.org/ticket/19958